-
Notifications
You must be signed in to change notification settings - Fork 483
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Blogpost for Introduction to Dashboard plugins #560
Blogpost for Introduction to Dashboard plugins #560
Conversation
title: "Introduction to OpenSearch Dashboard Plugins" | ||
authors: | ||
- ashwinpc | ||
- vemsarat |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't contribute much to deserve credits :). I believe you should be the sole author.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did one pass, couple of comments.
description: "Plugins are fundamental to how Opensearch works, and that similarity extends to OpenSearch Dashboards too..." | ||
--- | ||
|
||
Plugins are fundamental to how Opensearch works, and that similarity extends to OpenSearch Dashboards too. All major components and services used within Dashboards is a plugin. Similar to OpenSearch, additional functionality can also be added using external plugins. As a follow up the blog post on how plugins work for Opensearch, in this post we will explore how plugins work for OpenSearch Dashboards. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that -> the
Not a grammatical expert, but some how it feels the
would be better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"All major components and services used within Dashboards is a plugin"
As a reader I don't know what these components and services are. It would be helpful to explain them.
The sentence starts with plural forms and converges with singular form. Probably something like ... Dashboards are plugins.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"additional functionality can also be added using external plugins"
Do we have internal and external plugins? Are they treated differently across the system?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback Sarat, I'll incorporate this feedback with any others in the next revision.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've redone this paragraph to make it easier to understand while keeping it short so that i can focus on what a plugin is sooner.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Super good blog post!
Most of my comments are style.
A few general comments:
- Should you say that this is in typescript at the start?
- There is no mention of packaging a plugin for distribution. Is there anything that can be said about that?
|
||
### What is a plugin | ||
|
||
First lets understand what an Opensearch Dashboards plugin is. In Opensearch Dashboards, plugins are classes that can be loaded via the [Dashboards plugin api](https://github.com/opensearch-project/OpenSearch-Dashboards/blob/1.2/src/core/CONVENTIONS.md#plugin-structure) to integrate with the core system via lifecycle events. They can consist of a client side code (public), server side code (server), or both. Plugins can also interact on each other and core from both places. Plugins must also contain a manifest file that describes a set of properties, both required and optional that core system can use to load and initialize the plugin correctly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- 'Openseach' -> 'OpenSearch'
- 'lets' should be 'let's' but that is a contraction for 'let us' which is not ideal. Maybe reframe the sentence to: "First, it's important to establish what a OpenSearch Dashboard plugin is'
- 'api' -> 'API'
- Is there a link you can provide to what a lifecycle event is?
- 'They' means lifecycle events? It's not clear. I'd be more explicit.
- '(public)' and '(server)' - are these some sort of reserved word?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'Openseach' -> 'OpenSearch'
done
'lets' should be 'let's' but that is a contraction for 'let us' which is not ideal. Maybe reframe the sentence to: "First, it's important to establish what a OpenSearch Dashboard plugin is'
done
'api' -> 'API'
done
Is there a link you can provide to what a lifecycle event is?
Unfortunately we dont have any documentation i can point to about it, but ive talked about it later in the post. Ive updated the text to link to that section
'They' means lifecycle events? It's not clear. I'd be more explicit.
done
'(public)' and '(server)' - are these some sort of reserved word?
Kinda. They are the reserved names for the folder for client side and server side code. (as shown in the folder structure below)
Created a new issue #619 to track this PR. @stockholmux will update the post in a slightly different format keeping in mind your feedback. |
@ashwin-pc Are you going to update this PR or create a new one? Not sure how much your blog post will change. |
I'm going to update this PR to preserve the comments. Hopefully not a lot. Mostly just the structure. |
|
9dd7741
to
59f3b56
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing big - a few small changes and clarifications needed
|
||
The manifest file signature is defined by the interface [`PluginManifest`](https://github.com/opensearch-project/OpenSearch-Dashboards/blob/1.2/src/core/server/plugins/types.ts#L126-L196) | ||
|
||
e.g. manifest file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
drop this line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to be sure, you're asking me to drop only line 76 right?
Signed-off-by: Ashwin Pc <ashwinpc@amazon.com>
Signed-off-by: Ashwin Pc <ashwinpc@amazon.com>
Signed-off-by: Ashwin Pc <ashwinpc@amazon.com>
587a738
to
2b6d9ae
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Awesome post @ashwin-pc! It really came together. I'm going to queue this up for tomorrow (1/21). I'll merge it then. |
+1 I really loved reading through it! |
Signed-off-by: Ashwin Pc ashwinpc@amazon.com
Description
Follow-up post to #526 covering Opensearch Dashboard plugins this time
Issues Resolved
#446
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the BSD-3-Clause License.